Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove tunnel state wrapper #5291

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Remove tunnel state wrapper #5291

merged 1 commit into from
Oct 17, 2023

Conversation

dlon
Copy link
Member

@dlon dlon commented Oct 14, 2023

Minor cleanup that gets rid of the state_wrapper macro in favor of boxing the TunnelState trait.

enum EventConsequence {
    NewState((Box<dyn TunnelState>, TunnelStateTransition)),
    SameState(Box<dyn TunnelState>),
    Finished,
}

trait TunnelState: Send {
    fn handle_event(
        self: Box<Self>,
        runtime: &tokio::runtime::Handle,
        commands: &mut TunnelCommandReceiver,
        shared_values: &mut SharedTunnelStateValues,
    ) -> EventConsequence;
}

replaces

enum EventConsequence {
    NewState((TunnelStateWrapper, TunnelStateTransition)),
    SameState(TunnelStateWrapper),
    Finished,
}

trait TunnelState: Into<TunnelStateWrapper> + Sized {
    type Bootstrap;

    fn enter(
        shared_values: &mut SharedTunnelStateValues,
        bootstrap: Self::Bootstrap,
    ) -> (TunnelStateWrapper, TunnelStateTransition);

    fn handle_event(
        self,
        runtime: &tokio::runtime::Handle,
        commands: &mut TunnelCommandReceiver,
        shared_values: &mut SharedTunnelStateValues,
    ) -> EventConsequence;
}

macro_rules! state_wrapper {
    (enum $wrapper_name:ident { $($state_variant:ident($state_type:ident)),* $(,)* }) => {
        enum $wrapper_name {
            $($state_variant($state_type),)*
        }

        $(impl From<$state_type> for $wrapper_name {
            fn from(state: $state_type) -> Self {
                $wrapper_name::$state_variant(state)
            }
        })*

        impl $wrapper_name {
            fn handle_event(
                self,
                runtime: &tokio::runtime::Handle,
                commands: &mut TunnelCommandReceiver,
                shared_values: &mut SharedTunnelStateValues,
            ) -> EventConsequence {
                match self {
                    $($wrapper_name::$state_variant(state) => {
                        state.handle_event(runtime, commands, shared_values)
                    })*
                }
            }
        }
    }
}

state_wrapper! {
    enum TunnelStateWrapper {
        Disconnected(DisconnectedState),
        Connecting(ConnectingState),
        Connected(ConnectedState),
        Disconnecting(DisconnectingState),
        Error(ErrorState),
    }
}

.


This change is Reviewable

Copy link
Contributor

@MarkusPettersson98 MarkusPettersson98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dlon)


talpid-core/src/tunnel_state_machine/disconnected_state.rs line 16 at r1 (raw file):

/// No tunnel is running.
pub struct DisconnectedState(());

This could safely be defined as a simple unit struct:

pub struct DisconnectedState;

Code quote:

pub struct DisconnectedState(());

@dlon dlon force-pushed the cleanup-tunnel-state-transitions branch from b7ad42f to d668753 Compare October 17, 2023 12:51
@dlon dlon merged commit e166916 into main Oct 17, 2023
25 checks passed
@dlon dlon deleted the cleanup-tunnel-state-transitions branch October 17, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants